Skip to content

Follow up IME option creation and delete dialog timing fixes#3185

Merged
Chartman123 merged 1 commit intonextcloud:mainfrom
don9x2E:fix/ime-option-creation-followup
Mar 3, 2026
Merged

Follow up IME option creation and delete dialog timing fixes#3185
Chartman123 merged 1 commit intonextcloud:mainfrom
don9x2E:fix/ime-option-creation-followup

Conversation

@don9x2E
Copy link
Contributor

@don9x2E don9x2E commented Feb 26, 2026

Summary

Follow-up fixes after #3183 to address remaining real-world Korean IME issues and related UX regressions.

  • Prevent unintended option creation while IME composition is active
  • Local "Add a new answer option" row now creates only on explicit intent:
    • Enter after composition end, or
    • explicit + button click
  • Keep live update behavior for already-created options
  • After creating an option with Enter, move focus to the next empty local option row
  • Fix delayed "Delete form" confirmation dialog by rendering dialog outside the actions slot

Why this follow-up is needed

The initial IME patch improved composition handling, but in actual Korean IME use there were still edge cases where input-driven side effects could trigger option creation timing unexpectedly.

Switching local-row creation to explicit intent removes those composition timing race paths while preserving normal editing for existing options.

Trade-off

Auto-create-on-input for the local empty row is intentionally disabled for now to prioritize deterministic IME-safe behavior.

Validation

  • IME composition does not auto-create options
  • Exactly one option is created on explicit Enter/click
  • Focus advances to the next empty local row after creation
  • Delete-form confirmation opens immediately on click

Copy link
Collaborator

@Chartman123 Chartman123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just some small comments :)

@Chartman123 Chartman123 requested a review from susnux February 27, 2026 12:39
@Chartman123 Chartman123 added bug Something isn't working enhancement New feature or request javascript Javascript related ticket 3. to review Waiting for reviews feature: 📑 form creation labels Feb 27, 2026
@don9x2E
Copy link
Contributor Author

don9x2E commented Feb 27, 2026

Thanks for the review!

  • I’ll move the AppNavigationForm.vue formatting change into a separate PR that references Delete form only possible by super weird, click order #3099.

  • I’ll update the add option button to be disabled only during active IME composition (so it works for direct inputs).

  • I’ll add the missing JSDoc blocks for the new handlers.

  • I’ll push the follow-up updates shortly.

@don9x2E don9x2E force-pushed the fix/ime-option-creation-followup branch from cf90b95 to 972f2ae Compare February 27, 2026 13:35
@Chartman123
Copy link
Collaborator

@don9x2E now the button is always enabled, even if there's no input at all. 😉

@susnux Can't we get rid of the debounce logic for onInput if we only submit the new options upon pressing enter or clicking the button?

@don9x2E don9x2E force-pushed the fix/ime-option-creation-followup branch from bb59d34 to 34b3ea4 Compare February 28, 2026 00:23
@don9x2E
Copy link
Contributor Author

don9x2E commented Feb 28, 2026

Addressed: the add button is now disabled when input is empty (kept enabled for non-empty input, only disabled during active IME composition).

Re debounce: we still need it for updating existing options (typing in non-local options triggers PATCH).

For new option creation, it only happens via Enter or the add button.

@Chartman123
Copy link
Collaborator

@don9x2E the button is still not working as expected: now it stays disabled even if there is some input. It looks like your canCreateLocalAnswer is not executed upon input.

@Chartman123
Copy link
Collaborator

@don9x2E still the same... there were some issues with reactivity left. I tried to fix it with Copilot. Please have a look at my commit, if it's still working correctly during IME input. non-IME input is working now.

Before we merge it, please rebase your code on current main and also squash all commits into a single one.

@susnux
Copy link
Collaborator

susnux commented Mar 2, 2026

Can't we get rid of the debounce logic for onInput if we only submit the new options upon pressing enter or clicking the button?

I guess so

@don9x2E don9x2E force-pushed the fix/ime-option-creation-followup branch from 04479c9 to e6dd00a Compare March 2, 2026 14:17
@don9x2E
Copy link
Contributor Author

don9x2E commented Mar 2, 2026

Rebased on current main and squashed into a single commit (e6dd00a) with DCO sign-off. Also removed the AppNavigationForm changes (handled separately in #3188).

@Chartman123
Copy link
Collaborator

@don9x2E you've lost my changes

@don9x2E don9x2E force-pushed the fix/ime-option-creation-followup branch from e6dd00a to c39eb6f Compare March 2, 2026 14:53
@don9x2E
Copy link
Contributor Author

don9x2E commented Mar 2, 2026

Sorry for the missing. Re-applied the reviewer changes and force-pushed the squashed commit (c39eb6f).

@Chartman123
Copy link
Collaborator

ok, now just the lint error and then we're good to go :)

Signed-off-by: don9x2E <revan@kakao.com>
@don9x2E don9x2E force-pushed the fix/ime-option-creation-followup branch from c39eb6f to e92a705 Compare March 3, 2026 09:41
@Chartman123 Chartman123 merged commit 5a68239 into nextcloud:main Mar 3, 2026
46 checks passed
@Chartman123
Copy link
Collaborator

/backport to stable5.2

@backportbot backportbot bot added the backport-request Pending backport by the backport-bot label Mar 3, 2026
@backportbot backportbot bot removed the backport-request Pending backport by the backport-bot label Mar 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews bug Something isn't working enhancement New feature or request feature: 📑 form creation javascript Javascript related ticket

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants